-
Notifications
You must be signed in to change notification settings - Fork 1
[INC-300] baton-github: optimize Grants() for org Resource #97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| bag, page, err := parsePageToken(pToken.Token, resource.Id) | ||
| if err != nil { | ||
| return nil, "", nil, err | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to use bag. But looks like we need to push resourceTypes to bag. bag.push(resourceTypes)
How to handle pagination nicely? in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The resource.Id should contain the resource type, right? You should be able to push page tokens for different resources and resource types, then pop stuff off and switch based on the resource type. See https://github.com/ConductorOne/baton-microsoft-entra/blob/main/pkg/connector/enterprise_applications.go#L267 for an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes need to use pagination bag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if rId == orgRoleAdmin { | ||
| rv = append(rv, o.orgRoleGrant(orgRoleMember, resource, ur.Id, user.GetID())) | ||
| } | ||
| rv = append(rv, o.orgRoleGrant(rId, resource, ur.Id, user.GetID())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Pending Memberships Included in Grants
The optimization removed the check for pending organization memberships. Previously, users with pending invitations were explicitly skipped. The new code, using role-filtered ListMembers() API calls, no longer performs this check, leading to pending members being incorrectly included in grants.

Uh oh!
There was an error while loading. Please reload this page.